feat: aggregated metric volume queries #14412

trevorwhitney · 2024-10-07T19:47:48Z

What this PR does / why we need it:

This PR introduces the ?aggregatedMetrics=true query parameter to the /volume endpoint which allows one to get stream volume from aggregated metrics rather than the index.

Explore Logs uses the /volume endpoint to find the top services by volume for the services landing page. This is slow for big services. With the latest release of Explore Logs, we introduced the ability to get this volume using aggregated metric streams, which is much faster. In that first release, however, the app is getting volumes for the aggregated metric streams rather than the actual service. This PR adds aggregated metric support directly in the the Loki /volume endpoint. This allows the frontend to send the same stream selector whether it is using aggregated metrics or not (and therefore not need to know about the __aggregated_metric__ label).

Which issue(s) this PR fixes:
Fixes #14349

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
Title matches the required conventional commits format, see here
- Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

trevorwhitney · 2024-10-07T20:14:49Z

It may be interesting to change the underlying instant query to a shardable topk query to enforce the requested limit once #14243 is merged

MasslessParticle · 2024-10-07T21:40:26Z

pkg/querier/queryrange/volume_test.go

+		"foo",
+	)
+	require.NoError(t, err)
+	fruitMatcher, err := labels.NewMatcher(labels.MatchNotEqual, "fruit", "apple")


Check out labels.MustNewMatcher(), it'll help make these tests less verbose

MasslessParticle · 2024-10-07T21:41:34Z

pkg/querier/queryrange/volume.go

+	filters := []string{}
+	serviceNamePresent := false
+
+	for i, matcher := range a.matchers {


Is len(matchers) ever 0? If not, it might be good to pull the if i == 0 clause out for clarity

MasslessParticle · 2024-10-07T21:45:35Z

pkg/loghttp/query.go

+	Limit             uint32
+	TargetLabels      []string
+	AggregateBy       string
+	AggregatedMetrics bool


nit: I tripped over the naming here. It might be better to use a name like UseAggregatedMetrics or FromAggregatedMetrics

shantanualsi · 2024-10-08T08:35:54Z

pkg/querier/queryrange/roundtrip.go

+			// idxVolumeHandler gets volume from metadata in the index
+			idxVolumeHandler := indexTw.Wrap(next)
+			if !r.AggregatedMetrics || !cfg.AggregatedMetrics {
+				return idxVolumeHandler.Do(ctx, req)


is it intended to silently not use aggregated metrics when the user explicitly sets "aggregatedMetrics=true" in the request, but does not have patterns enabled?
or should we throw a bad request instead?

…14413) This change allows to observe the distribution of how many tasks are dequeued at once over time. Signed-off-by: Christian Haudum <[email protected]>

…ext (#14420)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…14418)

At the moment, errors are silently ignored when asynchronous blocks downloading fails. Signed-off-by: Christian Haudum <[email protected]>

…o v4.24.9 (#14357)" (#14437)

**Statefulsets for ingester and index-gateway** The change here is to make the `spec.updateStrategy.type` configurable, and to default to `RollingUpdate`. **Enterprise tokengen** For this change, if `enterprise` and `tokengen` are enabled, and `rbac.namespaced` is true, the chart will render a `Role` and `RoleBinding`. If `rbac.namespaced` is false, it will render a `ClusterRole` and `ClusterRoleBinding`.

) This new field reports how many blocks have been processed in total in the multiplexed request. Signed-off-by: Christian Haudum <[email protected]>

Co-authored-by: Jack Baldry <[email protected]>

Signed-off-by: Vladyslav Diachenko <[email protected]> Co-authored-by: Trevor Whitney <[email protected]>

Co-authored-by: J Stickler <[email protected]>

github-actions · 2024-10-10T21:07:26Z

Kubernetes Manifest Diff Summary

Scenario: default-single-binary-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: default-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: ingress-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: legacy-monitoring-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: simple-scalable-aws-kube-irsa-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

trevorwhitney · 2024-10-10T21:27:51Z

at this time, I don't think this is worth merging. over large time ranges (like 7d), this is significantly slower than getting the same data from the index. granted it's more accurate, but accuracy is not what we need for Explore Logs. there may be use in bringing this back for cost attribution or Logs Volume Explorer, but I think it's premature to merge this for that purpose at this time.

trevorwhitney requested a review from a team as a code owner October 7, 2024 19:47

pull-request-size bot added the size/XL label Oct 7, 2024

trevorwhitney added 2 commits October 7, 2024 14:11

feat: aggregated metric volume queries

93cc007

feat: add range support to agg metric volume queries

c78c5fa

trevorwhitney force-pushed the volume-agg-metrics branch from b24af03 to c78c5fa Compare October 7, 2024 20:11

chore: lint and format

55b0aa1

MasslessParticle reviewed Oct 7, 2024

View reviewed changes

MasslessParticle approved these changes Oct 7, 2024

View reviewed changes

shantanualsi reviewed Oct 8, 2024

View reviewed changes

trevorwhitney and others added 18 commits October 8, 2024 13:18

chore: fix tests, add more requestion validation

4ea549a

chore: Make metric for dequeued tasks in bloom-gateway a Histogram (#…

2135959

…14413) This change allows to observe the distribution of how many tasks are dequeued at once over time. Signed-off-by: Christian Haudum <[email protected]>

fix(storage/chunk/client/aws): have GetObject check for canceled cont…

103e020

…ext (#14420)

fix(kafka): Set namespace for Loki kafka metrics (#14426)

d5ce63e

docs: Updated Promtail to Alloy (#14404)

0592591

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

feat(kafka): Enable querier to optionally query partition ingesters (#…

0ee464f

…14418)

chore: Log errors when processing a download task fails (#14436)

7e589db

At the moment, errors are silently ignored when asynchronous blocks downloading fails. Signed-off-by: Christian Haudum <[email protected]>

fix: Revert "fix(deps): update module github.com/shirou/gopsutil/v4 t…

35bca10

…o v4.24.9 (#14357)" (#14437)

chore: Rename new querier flag to use dashes (#14438)

8963b0e

chore(operator): Update build and runtime deps (#14416)

964928d

chore: Add new field to "stats-report" log line in bloom gateway (#14446

3d500b8

) This new field reports how many blocks have been processed in total in the multiplexed request. Signed-off-by: Christian Haudum <[email protected]>

docs: remove reference to Agent Flow (#14449)

92bae79

docs: Revise the LogQL Analyzer topic (#14374)

8e94ee6

Co-authored-by: Jack Baldry <[email protected]>

fix(ci): updated helm diff rendering workflow (#14424)

b4d2567

Signed-off-by: Vladyslav Diachenko <[email protected]> Co-authored-by: Trevor Whitney <[email protected]>

docs: Update alloy-otel-logs.md to correct a typo (#13827)

5dadb6d

Co-authored-by: J Stickler <[email protected]>

fix: level detection for warning level (#14444)

887db47

ci: speciy golangci-lint build tags at runtime (#14456)

edcd09a

trevorwhitney added 2 commits October 10, 2024 14:03

fix: nix build, downgrade toolchain to go1.23.1 (#14442)

f9213a2

fix: always do a range query

9d29b15

trevorwhitney requested review from periklis, xperimental and JoaoBraveCoding as code owners October 10, 2024 21:06

github-actions bot added sig/operator area/helm type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories labels Oct 10, 2024

pull-request-size bot added size/XXL and removed size/XL labels Oct 10, 2024

trevorwhitney closed this Oct 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: aggregated metric volume queries #14412

feat: aggregated metric volume queries #14412

trevorwhitney commented Oct 7, 2024 •

edited

Loading

trevorwhitney commented Oct 7, 2024

MasslessParticle Oct 7, 2024

MasslessParticle Oct 7, 2024

MasslessParticle Oct 7, 2024

shantanualsi Oct 8, 2024

github-actions bot commented Oct 10, 2024

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

trevorwhitney commented Oct 10, 2024

feat: aggregated metric volume queries #14412

feat: aggregated metric volume queries #14412

Conversation

trevorwhitney commented Oct 7, 2024 • edited Loading

trevorwhitney commented Oct 7, 2024

MasslessParticle Oct 7, 2024

Choose a reason for hiding this comment

MasslessParticle Oct 7, 2024

Choose a reason for hiding this comment

MasslessParticle Oct 7, 2024

Choose a reason for hiding this comment

shantanualsi Oct 8, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2024

Kubernetes Manifest Diff Summary

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

trevorwhitney commented Oct 10, 2024

trevorwhitney commented Oct 7, 2024 •

edited

Loading